Replace some PEP references with internal references #1405
Conversation
The versions of pip and setuptools in question date back to 2014.
jeanas
force-pushed
the
pep-references
branch
from
1dad7b1
to
b746b22
Compare
|
I have fixed a bunch of merge conflicts with |
jeanas
force-pushed
the
pep-references
branch
from
b746b22
to
79385f3
Compare
source/glossary.rst
Outdated
| <pyproject-guide-build-system-table>` of their ``pyproject.toml`` file | ||
| (or the deprecated practice of having no ``pyproject.toml`` but either | ||
| ``setup.cfg`` or ``setup.py``), another practical way to define projects |
There was a problem hiding this comment.
While in it, please use the :file: role for inline highlighting.
Also, the new text implies that having just setup.cfg works. But it doesn't. It only works in pair with either PEP 517 or the deprecated setup.py CLI invocation. Otherwise, there's simply nothing that would attempt reading it.
There was a problem hiding this comment.
There are less ocurrences of :file:`pyproject.toml` than ``pyproject.toml``. I'd rather keep this PR focused on PEP references, we can make it more consistent globally.
There was a problem hiding this comment.
Good remark about setup.cfg, it looks like neither pip nor build accepts a source tree without either pyproject.toml or setup.py. I'll change this.
There was a problem hiding this comment.
There are less ocurrences of
:file:`pyproject.toml`thanpyproject.toml. I'd rather keep this PR focused on PEP references, we can make it more consistent globally.
True for the old content, but I think that when adding new content, we could use the better role right away, though.
|
The CI failure is a spurious linkcheck failure. Edit: in fact the workflow triggered by the PR succeeded, although the one triggered by the push failed. |
|
@webknjaz Anything missing here? |
|
(general contribution process feedback) @jeanas sorry, I make review rounds once in a while and sometimes miss things. Plus, it's hard to find the silently resolved threads, which results in more effort required to find and go through them (this is the primary reason for having thread authors resolve them or causing resolution notifications at least). |
| - Corresponding :ref:`core metadata <core-metadata>` field: | ||
| :ref:`Requires-Dist <core-metadata-requires-dist>` and | ||
| :ref:`Provides-Extra <core-metadata-provides-extra>` | ||
|
|
||
| The (optional) dependencies of the project. | ||
|
|
||
| For ``dependencies``, it is a key whose value is an array of strings. |
There was a problem hiding this comment.
(general contribution process feedback)
Try not to reflow lines that don't change the content: it's hard to compare what changed, if an entire block of 13 lines is marked as updated — this causes an unnecessary cognitive load for the reviewers trying to compare strings byte-by-byte...
This is the reason it's recommended to split the formatting changes from the functional ones in normal code projects, but I'm sure it's also applicable to the docs.
Here's a collection of materials related to review processes, I enjoyed reading — maybe you'll find some insights for yourself too:
| :ref:`Requires-Dist <core-metadata-requires-dist>` entry for the | ||
| matching :ref:`Provides-Extra <core-metadata-provides-extra>` | ||
| metadata. | ||
| For ``dependencies``, it is a key whose value is an array of strings. Each |
There was a problem hiding this comment.
Note for myself: this line doesn't have content changes, it just has an extra trailing word that was previously on the following line.
| matching :ref:`Provides-Extra <core-metadata-provides-extra>` | ||
| metadata. | ||
| For ``dependencies``, it is a key whose value is an array of strings. Each | ||
| string represents a dependency of the project and MUST be formatted as a valid |
There was a problem hiding this comment.
Note for myself: no changes here either.
| metadata. | ||
| For ``dependencies``, it is a key whose value is an array of strings. Each | ||
| string represents a dependency of the project and MUST be formatted as a valid | ||
| :ref:`dependency specifier <dependency-specifiers>`. Each string maps directly |
There was a problem hiding this comment.
Note for myself: the actual update from :pep: to :ref:. Plus an inter-sentence whitespace doubled.
| For ``dependencies``, it is a key whose value is an array of strings. Each | ||
| string represents a dependency of the project and MUST be formatted as a valid | ||
| :ref:`dependency specifier <dependency-specifiers>`. Each string maps directly | ||
| to a :ref:`Requires-Dist <core-metadata-requires-dist>` entry. |
There was a problem hiding this comment.
Note for myself: no content changes.
| :ref:`dependency specifier <dependency-specifiers>`. Each string maps directly | ||
| to a :ref:`Requires-Dist <core-metadata-requires-dist>` entry. | ||
|
|
||
| For ``optional-dependencies``, it is a table where each key specifies an extra |
There was a problem hiding this comment.
Note for myself: no content changes here, it just moved two extra trailing words from the following line.
| to a :ref:`Requires-Dist <core-metadata-requires-dist>` entry. | ||
|
|
||
| For ``optional-dependencies``, it is a table where each key specifies an extra | ||
| and whose value is an array of strings. The strings of the arrays must be valid |
There was a problem hiding this comment.
Note for myself: no content changes here, just reflow.
|
|
||
| For ``optional-dependencies``, it is a table where each key specifies an extra | ||
| and whose value is an array of strings. The strings of the arrays must be valid | ||
| :ref:`dependency specifiers <dependency-specifiers>`. The keys MUST be valid |
There was a problem hiding this comment.
Note for myself: ref update; double whitespace.
| values for :ref:`Provides-Extra <core-metadata-provides-extra>`. Each value in | ||
| the array thus becomes a corresponding :ref:`Requires-Dist | ||
| <core-metadata-requires-dist>` entry for the matching :ref:`Provides-Extra | ||
| <core-metadata-provides-extra>` metadata. |
There was a problem hiding this comment.
Note for myself: reflow; double whitespaces.
| in :pep:`685`. For tools writing the file, it is recommended only to insert a space | ||
| between the object reference and the left square bracket. |
| extras is formally specified in the :ref:`dependency specifier specification | ||
| <dependency-specifiers>` (as ``extras``) and restrictions on values specified |
| @@ -102,7 +102,7 @@ not only provides features that plain :ref:`distutils` doesn't offer | |||
| also provides a consistent build interface and feature set across all | |||
| supported Python versions. | |||
|
|
|||
| Consequently, :ref:`distutils` was deprecated in Python 3.10 by :pep:`632` and | |||
| Consequently, :ref:`distutils` was deprecated in Python 3.10 (by :pep:`632`) and | |||
There was a problem hiding this comment.
Note for myself: style change — parens.
| namespace package is that you just omit :file:`__init__.py` from the | ||
| namespace package directory. An example file structure (following | ||
| :ref:`src-layout <setuptools:src-layout>`): |
| :ref:`setuptools`, another practical way to define projects currently | ||
| is something that contains a :term:`pyproject.toml`, :term:`setup.py`, | ||
| or :term:`setup.cfg` file at the root of the project source directory. | ||
| Since projects create :term:`Distributions <Distribution Package>` using |
There was a problem hiding this comment.
Note to myself: removed "most" + reflow.
source/glossary.rst
Outdated
| projects currently is something that contains a :term:`pyproject.toml` | ||
| or :term:`setup.py` file at the root of the project source directory. |
There was a problem hiding this comment.
Can we improve this so that it doesn't imply a suggestion to use setup.py w/o pyproject.toml? Also, I'd really like a mention of setup.cfg to be kept in the context of declarative metadata.
There was a problem hiding this comment.
Just two lines before, it says “(or with the deprecated practice of having a setup.py without an accompanying pyproject.toml)”, is this is not enough? Maybe I should put “deprecated” in italics?
| even include multiple platform-specific Python distributions, meaning that a | ||
| single pex file can be portable across Linux and macOS. pex is released under the | ||
| Apache License 2.0. | ||
| `pex <https://pypi.org/project/pex/>`__ is a library for generating .pex |
There was a problem hiding this comment.
Note to myself: I don't understand what changed on this line. Using Ctrl+F highlights both old an new versions the same way. Can it be some non-printable whitespace? 🤔
There was a problem hiding this comment.
It's a double space removed in is a library. Let me split that in its own commit.
There was a problem hiding this comment.
(I ended up leaving it, it's not really important. It had been removed by my editor.)
| single pex file can be portable across Linux and macOS. pex is released under the | ||
| Apache License 2.0. | ||
| `pex <https://pypi.org/project/pex/>`__ is a library for generating .pex | ||
| (Python EXecutable) files which are executable Python environments in |
| Apache License 2.0. | ||
| `pex <https://pypi.org/project/pex/>`__ is a library for generating .pex | ||
| (Python EXecutable) files which are executable Python environments in | ||
| the spirit of virtualenvs. pex is an expansion upon the ideas found in |
There was a problem hiding this comment.
Note to myself: wording change
| applications as simple as cp. pex files may even include multiple | ||
| platform-specific Python distributions, meaning that a single pex file | ||
| can be portable across Linux and macOS. pex is released under the Apache | ||
| License 2.0. |
|
@jeanas another round of review complete. I marked most of the files as "viewed" on the GH UI so if you don't change them, they won't show up for me in the next rounds. I made some self-notes to help myself understand if there were any changes in some places, but those are non-actionable. I think the only concern is regarding the same paragraph: https://github.com/pypa/packaging.python.org/pull/1405/files#r1435296663 — reading the wording made me feel like it implies the use of |
Sorry — I never quite know what convention to use for this. On most projects that I personally work on, it is usual for PR authors to resolve threads that they (believe to) have addressed, so that it is easy to see what the outstanding issues are without writing "Done" on each thread. I think this varies widely between projects...
Ok, I will try to be more careful with this. (I have a compulsive habit of pressing M-q in Emacs.) |
jeanas
force-pushed
the
pep-references
branch
from
9d745ba
to
b96c506
Compare
|
@webknjaz I have greatly minimized the diff, let me know if it's better. |
Use internal references when the PEPs have been imported into the PUG.
jeanas
force-pushed
the
pep-references
branch
from
b96c506
to
b1c476c
Compare
|
I do not have the time to shepherd PRs at the moment; if anybody feels this is worth saving, please feel free to pick it up. |
Use internal references when the PEPs have been imported into the PUG.
📚 Documentation preview 📚: https://python-packaging-user-guide--1405.org.readthedocs.build/en/1405/